Re: [PATCH v22 00/25] Type2 device basic support

[<dan.j.williams@intel.com>]

alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
> 
> The patchset should be applied on the described base commit then applying
> Terry's v13 about CXL error handling. The first 3 patches come from Dan's
> for-6.18/cxl-probe-order branch with minor modifications.
> 
> This last version introduces support for Type2 decoder committed by
> firmware, implying CXL region automatically created during memdev
> initialization. New patches 11, 13 and 14 show this new core support
> with the sfc driver using it.

"Using" in what aspect? Does your test platform auto-create Type-2
regions? I know that is expected on the platforms PJ is using, but I
want to get a sense of what is the highest priority for Linux to address
first.

My sense, from the trouble PJ has been having, is that regions committed
by firmware is a higher priority than driver created regions.  Yes, the
subsystem will support both in the end, but in terms of staging this set
incrementally, I think we probably want to review one mode at a time.

> This driver has also support for the
> option used until today, where HDM decoders not committed. This is true
> under certain scenarios and also after the driver has been unload. This
> brings up the question if such firmware committer decoder should be
> reset at driver unload, assuming no locked HDM what this patchset does
> not support.

This question is asked and answered in Smita's Soft Reserve Recovery
effort. See this discussion [1]:

[1]:
http://lore.kernel.org/6930dacd6510f_198110020@dwillia2-mobl4.notmuch

The quick summary is that regions and decoders alive before the expander
or accelerator driver loaded should stay alive after the driver is
unloaded. Only explicit userspace driven de-commit can convert firmware
established regions back to driver established regions.

> v22 changes:
> 
>   patch 1-3 from Dan's branch without any changes.

Note for others following along, I am deleting that RFC branch in favor
of formal patches here [2].

[2]: http://lore.kernel.org/20251204022136.2573521-1-dan.j.williams@intel.com

The expectation is use that set to finish Smita's series [3]. Then
finalize the port and error handling rework's in Terry's (which will end
up removing mapped CXL component registers from 'struct cxl_dev_state),
and then queue this series on top.

[3]: http://lore.kernel.org/20251120031925.87762-1-Smita.KoralahalliChannabasappa@amd.com

>   patch 11: new
>   
>   patch 12: moved here from v21 patch 22
> 
>   patch 13-14: new
> 
>   patch 23: move check ahead of type3 only checks
> 
>   All patches with sfc changes adapted to support both options.

Going forward the log of old changes can be replaced with a link to the
N-1 posting.

Given the backlog of the Soft Reserve Recovery, CXL Protocol Error
Handling, and this Accelerator series I want to see some of these
precursor patches land on a topic branch before moving on to dealing
with the full accelerator set. I.e. I think we are at the point where
this can stop posting on moving baselines and focus on getting the the
dependencies into a topic branch in cxl.git.

[..]

How much of the changelog below is still relevant. It still talks about
the original RFC. Might it need a refresh given current learnings and
the passage of time? For example, no need to talk about CXL.cache, first
things first, just get non-cxl_pci based CXL.mem going.

> v2 changes:
> 
> I have removed the introduction about the concerns with BIOS/UEFI after the
> discussion leading to confirm the need of the functionality implemented, at
> least is some scenarios.
> 
> There are two main changes from the RFC:
> 
> 1) Following concerns about drivers using CXL core without restrictions, the CXL
> struct to work with is opaque to those drivers, therefore functions are
> implemented for modifying or reading those structs indirectly.
> 
> 2) The driver for using the added functionality is not a test driver but a real
> one: the SFC ethernet network driver. It uses the CXL region mapped for PIO
> buffers instead of regions inside PCIe BARs.
> 
> RFC:
> 
> Current CXL kernel code is focused on supporting Type3 CXL devices, aka memory
> expanders. Type2 CXL devices, aka device accelerators, share some functionalities
> but require some special handling.
> 
> First of all, Type2 are by definition specific to drivers doing something and not just
> a memory expander, so it is expected to work with the CXL specifics. This implies the CXL
> setup needs to be done by such a driver instead of by a generic CXL PCI driver
> as for memory expanders. Most of such setup needs to use current CXL core code
> and therefore needs to be accessible to those vendor drivers. This is accomplished
> exporting opaque CXL structs and adding and exporting functions for working with
> those structs indirectly.
> 
> Some of the patches are based on a patchset sent by Dan Williams [1] which was just
> partially integrated, most related to making things ready for Type2 but none
> related to specific Type2 support. Those patches based on Dan´s work have Dan´s
> signing as co-developer, and a link to the original patch.
> 
> A final note about CXL.cache is needed. This patchset does not cover it at all,
> although the emulated Type2 device advertises it. From the kernel point of view
> supporting CXL.cache will imply to be sure the CXL path supports what the Type2
> device needs. A device accelerator will likely be connected to a Root Switch,
> but other configurations can not be discarded. Therefore the kernel will need to
> check not just HPA, DPA, interleave and granularity, but also the available
> CXL.cache support and resources in each switch in the CXL path to the Type2
> device. I expect to contribute to this support in the following months, and
> it would be good to discuss about it when possible.
> 
> [1] https://lore.kernel.org/linux-cxl/98b1f61a-e6c2-71d4-c368-50d958501b0c@intel.com/T/
> 
[..]