Thread (68 messages) 68 messages, 6 authors, 2025-11-11

Re: [PATCH v19 18/22] cxl: Allow region creation by type2 drivers

From: Dave Jiang <dave.jiang@intel.com>
Date: 2025-10-20 13:59:09
Also in: linux-cxl


On 10/20/25 6:24 AM, Alejandro Lucero Palau wrote:
On 10/16/25 14:23, Cheatham, Benjamin wrote:
quoted
On 10/15/2025 4:42 PM, Dave Jiang wrote:
quoted
On 10/9/25 1:56 PM, Cheatham, Benjamin wrote:
quoted
On 10/6/2025 5:01 AM, alejandro.lucero-palau@amd.com wrote:
quoted
From: Alejandro Lucero <redacted>

Creating a CXL region requires userspace intervention through the cxl
sysfs files. Type2 support should allow accelerator drivers to create
such cxl region from kernel code.

Adding that functionality and integrating it with current support for
memory expanders.

Support an action by the type2 driver to be linked to the created region
for unwinding the resources allocated properly.

Based on https://lore.kernel.org/linux-cxl/168592159835.1948938.1647215579839222774.stgit@dwillia2-xfh.jf.intel.com/ (local)

Signed-off-by: Alejandro Lucero <redacted>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
Fix for this one should be split between 13/22 and this patch, but the majority of it is in this one. The idea is
if we don't find a free decoder we check for pre-programmed decoders and use that instead. Unfortunately, this
invalidates some of the assumptions made by __construct_new_region().
Wouldn't you look for a pre-programmed decoder first and construct the auto region before you try to manually create one? Also for a type 2 device, would the driver know what it wants and what the region configuration should look like? Would it be a single region either it's auto or manual, or would there be a configuration of multiple regions possible? To me a type 2 region is more intentional where the driver would know exactly what it needs and thus trying to get that from the cxl core.
Since this is a fix I didn't want to supersede the current behavior. A better solution would've been to add a flag to allow the type 2 driver
to set up an expected region type.

As for multiple regions, I have no clue. I haven't heard of any reason why a type 2 device would need multiple regions, but it's still very
early days. I don't think there's anything in this set that keeps you from using multiple regions though.

What Dave says is correct, so Type2 should  support these two possibilities, an HDM decoder already programmed by the BIOS and the BIOS doing nothing, at least with the Type2 HDM decoders. This patchset supports the latter, but the former is more than possible, even if the logic and what we have discussed since the RFC points to type2 driver having the full control.


However, I would prefer to do that other support as a follow-up as the functionality added is enough for the initial client, the sfc driver, requiring this new Type2 support. The reason is clear: I do not want to delay this "basic Type2 support" more than necessary, and as I stated in another comment, I bet we will see other things to support soon, so better to increasingly add those after a first work set the base. Of course, the base needs to be right.
I'm fine with the follow on approach. We probably should make a note somewhere in a commit log somewhere that only manual assemble mode is currently supported. And need to reject the BIOS created region and exit type2 CXL setup if present?

DJ

Thanks,

Alejandro

quoted
Thanks,
Ben
quoted
DJ
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help