Message-ID: <b472829d-c79e-4cf1-a789-427213fca28c@amd.com>
Date: Mon, 17 Mar 2025 07:55:42 +0000
From: Alejandro Lucero Palau <alucerop@....com>
To: Alison Schofield <alison.schofield@...el.com>,
 alejandro.lucero-palau@....com
Cc: linux-cxl@...r.kernel.org, netdev@...r.kernel.org,
 dan.j.williams@...el.com, edward.cree@....com, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com, dave.jiang@...el.com
Subject: Re: [PATCH v11 00/23] add type2 device basic support


On 3/12/25 17:57, Alison Schofield wrote:
> On Mon, Mar 10, 2025 at 09:03:17PM +0000, alejandro.lucero-palau@....com wrote:
>> From: Alejandro Lucero <alucerop@....com>
> Hi Alejandro,
>
> Can you restore a cover letter here?
> I'm particularly looking for more context around kernel driven region
> creation.


Opps, I did not want to have this dropping of previous versions and 
original RFC.


I will restore it properly for next version. Meanwhile, this is the 
missing text:


v10 changes:
  - Using cxl_memdev_state instead of cxl_dev_state for type2 which has a
    memory after all and facilitates the setup.
  - Adapt core for using cxl_memdev_state allowing accel drivers to work
    with them without further awareness of internal cxl structs.
  - Using last DPA changes for creating DPA partitions with accel driver
    hardcoding mds values when no mailbox.
  - capabilities not a new field but built up when current register maps
    is performed and returned to the caller for checking.
  - HPA free space supporting interleaving.
  - DPA free space droping max-min for a simple alloc size.

v9 changes:
  - adding forward definitions (Jonathan Cameron)
  - using set_bit instead of bitmap_set (Jonathan Cameron)
  - fix rebase problem (Jonathan Cameron)
  - Improve error path (Jonathan Cameron)
  - fix build problems with cxl region dependency (robot)
  - fix error path (Simon Horman)

v8 changes:
  - Change error path labeling inside sfc cxl code (Edward Cree)
  - Properly handling checks and error in sfc cxl code (Simon Horman)
  - Fix bug when checking resource_size (Simon Horman)
  - Avoid bisect problems reordering patches (Edward Cree)
  - Fix buffer allocation size in sfc (Simon Horman)

v7 changes:

  - fixing kernel test robot complains
  - fix type with Type3 mandatory capabilities (Zhi Wang)
  - optimize code in cxl_request_resource (Kalesh Anakkur Purayil)
  - add sanity check when dealing with resources arithmetics (Fan Ni)
  - fix typos and blank lines (Fan Ni)
  - keep previous log errors/warnings in sfc driver (Martin Habets)
  - add WARN_ON_ONCE if region given is NULL

v6 changes:

  - update sfc mcdi_pcol.h with full hardware changes most not related to
    this patchset. This is an automatic file created from hardware design
    changes and not touched by software. It is updated from time to time
    and it required update for the sfc driver CXL support.
  - remove CXL capabilities definitions not used by the patchset or
    previous kernel code. (Dave Jiang, Jonathan Cameron)
  - Use bitmap_subset instead of reinventing the wheel ... (Ben Cheatham)
  - Use cxl_accel_memdev for new device_type created (Ben Cheatham)
  - Fix construct_region use of rwsem (Zhi Wang)
  - Obtain region range instead of region params (Allison Schofield, Dave
    Jiang)

v5 changes:

  - Fix SFC configuration based on kernel CXL configuration
  - Add subset check for capabilities.
  - fix region creation when HDM decoders programmed by firmware/BIOS (Ben
    Cheatham)
  - Add option for creating dax region based on driver decission (Ben
    Cheatham)
  - Using sfc probe_data struct for keeping sfc cxl data

v4 changes:

  - Use bitmap for capabilities new field (Jonathan Cameron)

  - Use cxl_mem attributes for sysfs based on device type (Dave Jian)

  - Add conditional cxl sfc compilation relying on kernel CXL config 
(kernel test robot)

  - Add sfc changes in different patches for facilitating backport 
(Jonathan Cameron)

  - Remove patch for dealing with cxl modules dependencies and using sfc 
kconfig plus
    MODULE_SOFTDEP instead.

v3 changes:

  - cxl_dev_state not defined as opaque but only manipulated by accel 
drivers
    through accessors.

  - accessors names not identified as only for accel drivers.

  - move pci code from pci driver (drivers/cxl/pci.c) to generic pci code
    (drivers/cxl/core/pci.c).

  - capabilities field from u8 to u32 and initialised by CXL regs 
discovering
    code.

  - add capabilities check and removing current check by CXL regs 
discovering
    code.

  - Not fail if CXL Device Registers not found. Not mandatory for Type2.

  - add timeout in acquire_endpoint for solving a race with the endpoint 
port
    creation.

  - handle EPROBE_DEFER by sfc driver.

  - Limiting interleave ways to 1 for accel driver HPA/DPA requests.

  - factoring out interleave ways and granularity helpers from type2 region
    creation patch.

  - restricting region_creation for type2 to one endpoint decoder.

  - add accessor for release_resource.

  - handle errors and errors messages properly


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/

> --Alison
>
>> v11 changes:
>>   - Dropping the use of cxl_memdev_state and going back to using
>>     cxl_dev_state.
>>   - Using a helper for an accel driver to allocate its own cxl-related
>>     struct embedding cxl_dev_state.
>>   - Exporting the required structs in include/cxl/cxl.h for an accel
>>     driver being able to know the cxl_dev_state size required in the
>>     previously mentioned helper for allocation.
>>   - Avoid using any struct for dpa initialization by the accel driver
>>     adding a specific function for creating dpa partitions by accel
>>     drivers without a mailbox.
>>
>> Alejandro Lucero (23):
>>    cxl: add type2 device basic support
>>    sfc: add cxl support
>>    cxl: move pci generic code
>>    cxl: move register/capability check to driver
>>    cxl: add function for type2 cxl regs setup
>>    sfc: make regs setup with checking and set media ready
>>    cxl: support dpa initialization without a mailbox
>>    sfc: initialize dpa
>>    cxl: prepare memdev creation for type2
>>    sfc: create type2 cxl memdev
>>    cxl: define a driver interface for HPA free space enumeration
>>    fc: obtain root decoder with enough HPA free space
>>    cxl: define a driver interface for DPA allocation
>>    sfc: get endpoint decoder
>>    cxl: make region type based on endpoint type
>>    cxl/region: factor out interleave ways setup
>>    cxl/region: factor out interleave granularity setup
>>    cxl: allow region creation by type2 drivers
>>    cxl: add region flag for precluding a device memory to be used for dax
>>    sfc: create cxl region
>>    cxl: add function for obtaining region range
>>    sfc: update MCDI protocol headers
>>    sfc: support pio mapping based on cxl
>>
>>   drivers/cxl/core/core.h               |     2 +
>>   drivers/cxl/core/hdm.c                |    83 +
>>   drivers/cxl/core/mbox.c               |    30 +-
>>   drivers/cxl/core/memdev.c             |    47 +-
>>   drivers/cxl/core/pci.c                |   115 +
>>   drivers/cxl/core/port.c               |     8 +-
>>   drivers/cxl/core/region.c             |   411 +-
>>   drivers/cxl/core/regs.c               |    39 +-
>>   drivers/cxl/cxl.h                     |   112 +-
>>   drivers/cxl/cxlmem.h                  |   103 +-
>>   drivers/cxl/cxlpci.h                  |    23 +-
>>   drivers/cxl/mem.c                     |    26 +-
>>   drivers/cxl/pci.c                     |   118 +-
>>   drivers/cxl/port.c                    |     5 +-
>>   drivers/net/ethernet/sfc/Kconfig      |     9 +
>>   drivers/net/ethernet/sfc/Makefile     |     1 +
>>   drivers/net/ethernet/sfc/ef10.c       |    50 +-
>>   drivers/net/ethernet/sfc/efx.c        |    15 +-
>>   drivers/net/ethernet/sfc/efx_cxl.c    |   162 +
>>   drivers/net/ethernet/sfc/efx_cxl.h    |    40 +
>>   drivers/net/ethernet/sfc/mcdi_pcol.h  | 13645 +++++++++---------------
>>   drivers/net/ethernet/sfc/net_driver.h |    12 +
>>   drivers/net/ethernet/sfc/nic.h        |     3 +
>>   include/cxl/cxl.h                     |   269 +
>>   include/cxl/pci.h                     |    36 +
>>   tools/testing/cxl/Kbuild              |     1 -
>>   tools/testing/cxl/test/mock.c         |    17 -
>>   27 files changed, 6186 insertions(+), 9196 deletions(-)
>>   create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c
>>   create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h
>>   create mode 100644 include/cxl/cxl.h
>>   create mode 100644 include/cxl/pci.h
>>
>>
>> base-commit: 0a14566be090ca51a32ebdd8a8e21678062dac08
>> -- 
>> 2.34.1
>>
>>

Powered by blists - more mailing lists