Re: [RFC 1/2] cxl/memdev: add support for mutipf device

["Lucero Palau, Alejandro" <alejandro.lucero-palau@amd.com>]

On 28/08/2026 09:15, Richard Cheng wrote:
> On Thu, Aug 27, 2026 at 06:44:42PM +0800, Lucero Palau, Alejandro wrote:
>> On 25/08/2026 08:37, Lucero Palau, Alejandro wrote:
>>> On 24/08/2026 09:33, Richard Cheng wrote:
>>>> On Fri, Aug 21, 2026 at 04:51:33PM
>>>> +0800,alejandro.lucero-palau@amd.com wrote:
>>>>> From: Alejandro Lucero<alucerop@amd.com>
>>>>>
>>>>> A PCI device can present multiple Physical Functions(PFs) but the CXL
>>>>> specs restrict to the first one, PF0, the discovery and management of
>>>>> CXL capabilities accessed through a PF0 BAR. Other non-PF0 PFs need to
>>>>> obtain the CXL.mem range to work with somehow.
>>>>>
>>>>> Although this could be handled internally by an accelerator/Type2
>>>>> driver, it requires to properly handle changes to the CXL mem device,
>>>>> mainly its release by the CXL core, but also potential CXL device
>>>>> resets. When this release happens, those other PFs need to be
>>>>> told about
>>>>> it.
>>>>>
>>>>> Implement a way for non-PF0 PFs to register/unregister to the memdev
>>>>> linked to the PF0 device. At memdev release, trigger the release of
>>>>> those non-PF0 PFs devices registered to such memdev from the
>>>>> driver they
>>>>> are bound to.
>>>>>
>> <snip>
>>
>>>> I suggest replacing cxl_get_pf0_memdev() and cxl_put_pf0_memdev()
>>>> with another
>>>> helper, e.g.:
>>>>
>>>> int cxl_memdev_link_consumer(struct device *pf0, struct device
>>>> *consumer, struct range *range);
>>>>    It should live in cxl/core/memdev.c , and the behavior is
>>>> something like
>>>>
>>>> 1. Find PF0's memdev and take a temp ref.
>>>> 2. Lock the memdev
>>>> 3. Verify that the memdev is still registered, driver-bound,
>>>> attached, and has a valid HPA range
>>>> 4. Create a managed devce link via device_link_add(consumer,
>>>> &cxlmd->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
>>>
>>> Interesting approach.
>>>
>>>
>>> Not sure this could do the proper thing though.
>>> DL_FLAG_AUTOREMOVE_CONSUMER seems to remove the link, cxlmd->dev in your
>>> case, when consume driver unbinds ... but it is the other way what we
>>> need. Maybe I do not understand well all the implications with this
>>> approach, so let me study it.
>>
>> I'm having problems just trying to implement the supposedly basic
>> functionality linking the cxlmd device with the non-PF0 device, I mean
>> without thinking about potential races with this approach (I think it has
>> less problems in this regard than my approach).
>>
>>
>> I can use DL_FLAG_AUTOREMOVE_SUPPLIER with the supplier being cxlmd->dev, so
>> at device unbinding it can trigger the non-PF0 device unbinding as well. But
>> it seems all this link code is quite related to PM, so some checks at link
>> creation fail. I have tried using DL_FLAGS_SYNC_STATE_ONLY along with the
>> previous one, but another check precludes the link creation if both are used
>> (See device_link_flag_is_sync_state_only() ).
>>
>>
>> Do you have any advice here?
>>
>>
> Hi Alejandro,
>
> Thanks for trying the device-link approach. I wonder what deivce-link eperiment you actually execute ?
> Though I don't have your HW, maybe we can discuss on the experiment method ?
>
>  From what I can know from the current driver-core behavior is
> - DL_FALG_SYNC_STATE_ONLY | DL_FLAG_AUTOREMOVE_SUPPLIER is rejected by design
> - a sync-state-only link won't enforce consumer unbind
> - DL_FLAG_AUTOREMOVE_SUPPLIER alone is a valid flag combination, so a NULL there depends on runtime state or
>    the specific device relationship
>
> You're right that DL_FLAG_AUTOREMOVE_CONSUMER means the link is removed when the consumer driver unbinds.
> Now we get it more clear that, the autoremove flag control the liftetime of the link, they don't control supplier-to-consumer unbind direction.
>
> For normal managed device link, driver core unbinds active consumers before unbinding the supplier.
> For this case I think DL_FLAG_AUTOREMOVE_CONSUMER is appropriate since the non-PF0 driver creates the dependency during probe and no longer
> needs it after that driver unbinds.


I was confused with the flags. A non-PF0 function should be able to 
delete the link without triggering the unbinding of the supplier, and 
the supplier unbinding should trigger the non-PF0 unbinding. The flag 
seems to do the first one, and the implicit functionality when unbinding 
the supplier does the second thing. So, I think this could do what we need.

However,  I can not (properly) test it, because the supplier is checked 
with device_pm_initialized() and it fails for the memdev device and I 
guess it will with the region device as well. If I remove the check, it 
all works, so maybe adding some support for this case and conditionally 
do such PM check could be the way to go, as it simplifies a lot the 
design. I will study this further and see the implications.


Thanks!


> But I rethink about linking consumer to cxlmd->dev, memdev is not the object whose lifetime defines whether the returned HPA range is valid.
> An open /dev/cxl/memX can keep the memdev object alive even after the EP and region have been torn down.
>
> I am not sure but cxl_region seems like a more accurate supplier, it provides vaid HPA to consumers.
>
> I let GPT sketched the implementation of the API I was thinking about, maybe something like the following.
>
> """
>    int cxl_memdev_link_region_consumer(struct cxl_memdev *cxlmd,
>    				    struct device *consumer,
>    				    struct range *range)
>    {
>    	struct device *decoder_dev __free(put_device) = NULL;
>    	struct device *region_dev __free(put_device) = NULL;
>    	struct cxl_endpoint_decoder *cxled;
>    	struct cxl_region_params *p;
>    	struct cxl_region *cxlr = NULL;
>    	struct cxl_port *endpoint;
>    	struct device_link *link;
>
>    	endpoint = cxlmd->endpoint;
>    	if (!endpoint)
>    		return -EPROBE_DEFER;
>
>    	/*
>    	 * Endpoint removal owns region teardown, so this prevents the
>    	 * endpoint and its decoder children from disappearing while the
>    	 * supplier is being resolved.
>    	 */
>    	guard(device)(&endpoint->dev);
>
>    	if (!endpoint->dev.driver)
>    		return -EPROBE_DEFER;
>
>    	decoder_dev = device_find_child(&endpoint->dev, NULL,
>    					first_mapped_decoder);
>    	if (!decoder_dev)
>    		return -EPROBE_DEFER;
>
>    	cxled = to_cxl_endpoint_decoder(decoder_dev);
>
>    	/*
>    	 * Take an independent region-device reference while the decoder to
>    	 * region association is protected. The association is revalidated
>    	 * below after taking the region device lock.
>    	 */
>    	scoped_guard(rwsem_read, &cxl_rwsem.region) {
>    		cxlr = cxled->cxld.region;
>    		if (!cxlr)
>    			return -EPROBE_DEFER;
>
>    		region_dev = get_device(&cxlr->dev);
>    	}
>
>    	/*
>    	 * Serialize link creation against region driver unbind. Without this,
>    	 * a link could be added after device_links_busy() has already marked
>    	 * the supplier as unbinding and walked its existing consumers.
>    	 */
>    	guard(device)(region_dev);
>    	guard(rwsem_read)(&cxl_rwsem.region);
>    	guard(rwsem_read)(&cxl_rwsem.dpa);
>
>    	p = &cxlr->params;
>
>    	if (!region_dev->driver ||
>    	    cxled->cxld.region != cxlr ||
>    	    p->state != CXL_CONFIG_COMMIT ||
>    	    !p->res ||
>    	    p->nr_targets != 1)
>    		return -EPROBE_DEFER;
>
>    	link = device_link_add(consumer, region_dev,
>    			       DL_FLAG_AUTOREMOVE_CONSUMER);
>    	if (!link)
>    		return -ENXIO;
>
>    	*range = (struct range) {
>    		.start = p->res->start,
>    		.end = p->res->end,
>    	};
>
>    	return 0;
>    }
> """
>
> I think it needs more tweaks, but hope it can give you some idea.
>
> The intended ordering is then,
>
> non-PF0 probe -> validate comitted region -> create (consumer, region) managed link -> copy HPA range -> map the PF slice
> region teardown -> driver core sees an active/probing consumer -> wait -> unbind the non-PF0 driver -> consumer unmaps its HPA slice -> AUTOREMOVE_CONSUMER removes the link -> region teardown removes the decoder
>
> driver core can manage the device references and drops the supplier lock before forcing consumer unbind.
>
> Does it match the behavior you need ?
>
> Best regards,
> Richard Cheng.