Message-ID: <59fa7e55-f563-40f9-86aa-1873806e76cc@intel.com>
Date: Mon, 12 May 2025 15:36:27 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: alejandro.lucero-palau@....com, 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
Cc: Alejandro Lucero <alucerop@....com>
Subject: Re: [PATCH v15 00/22] Type2 device basic support



On 5/12/25 9:10 AM, alejandro.lucero-palau@....com wrote:
> From: Alejandro Lucero <alucerop@....com>
> 
> v15 changes:
>  - remove reference to unused header file (Jonathan Cameron)
>  - add proper kernel docs to exported functions (Alison Schofield)
>  - using an array to map the enums to strings (Alison Schofield)
>  - clarify comment when using bitmap_subset (Jonathan Cameron)
>  - specify link to type2 support in all patches (Alison Schofield)
> 
>   Patches changed (minor): 4, 11
>

Hi Alejandro,
Tried to pull this series using b4. Noticed couple things.
1. Can you run checkpatch on the entire series and fix any issues?
2. Can you rebase against v6.15-rc4? I think there are some conflicts against the fixes went in rc4.

Thanks!
 
> v14 changes:
>  - static null initialization of bitmaps (Jonathan Cameron)
>  - Fixing cxl tests (Alison Schofield)
>  - Fixing robot compilation problems
> 
>   Patches changed (minor): 1, 4, 6, 13
> 
> v13 changes:
>  - using names for headers checking more consistent (Jonathan Cameron)
>  - using helper for caps bit setting (Jonathan Cameron)
>  - provide generic function for reporting missing capabilities (Jonathan Cameron)
>  - rename cxl_pci_setup_memdev_regs to cxl_pci_accel_setup_memdev_regs (Jonathan Cameron)
>  - cxl_dpa_info size to be set by the Type2 driver (Jonathan Cameron)
>  - avoiding rc variable when possible (Jonathan Cameron)
>  - fix spelling (Simon Horman)
>  - use scoped_guard (Dave Jiang)
>  - use enum instead of bool (Dave Jiang)
>  - dropping patch with hardware symbols
>  
> v12 changes:
>  - use new macro cxl_dev_state_create in pci driver (Ben Cheatham)
>  - add public/private sections in now exported cxl_dev_state struct (Ben
>    Cheatham)
>  - fix cxl/pci.h regarding file name for checking if defined
>  - Clarify capabilities found vs expected in error message. (Ben
>    Cheatham)
>  - Clarify new CXL_DECODER_F flag (Ben Cheatham)
>  - Fix changes about cxl memdev creation support moving code to the
>    proper patch. (Ben Cheatham)
>  - Avoid debug and function duplications (Ben Cheatham)
>  - Fix robot compilation error reported by Simon Horman as well.
>  - Add doc about new param in clx_create_region (Simon Horman).
> 
> 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.
> 
> 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/
> 
> Alejandro Lucero (22):
>   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
>   sfc: 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: support pio mapping based on cxl
> 
>  drivers/cxl/core/core.h               |   2 +
>  drivers/cxl/core/hdm.c                |  86 +++++
>  drivers/cxl/core/mbox.c               |  37 ++-
>  drivers/cxl/core/memdev.c             |  47 ++-
>  drivers/cxl/core/pci.c                | 162 ++++++++++
>  drivers/cxl/core/port.c               |   8 +-
>  drivers/cxl/core/region.c             | 433 +++++++++++++++++++++++---
>  drivers/cxl/core/regs.c               |  40 ++-
>  drivers/cxl/cxl.h                     | 111 +------
>  drivers/cxl/cxlmem.h                  | 103 +-----
>  drivers/cxl/cxlpci.h                  |  23 +-
>  drivers/cxl/mem.c                     |  25 +-
>  drivers/cxl/pci.c                     | 111 ++-----
>  drivers/cxl/port.c                    |   5 +-
>  drivers/net/ethernet/sfc/Kconfig      |  10 +
>  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    | 159 ++++++++++
>  drivers/net/ethernet/sfc/efx_cxl.h    |  40 +++
>  drivers/net/ethernet/sfc/net_driver.h |  12 +
>  drivers/net/ethernet/sfc/nic.h        |   3 +
>  include/cxl/cxl.h                     | 292 +++++++++++++++++
>  include/cxl/pci.h                     |  36 +++
>  tools/testing/cxl/Kbuild              |   1 -
>  tools/testing/cxl/test/mem.c          |   3 +-
>  tools/testing/cxl/test/mock.c         |  17 -
>  27 files changed, 1415 insertions(+), 417 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: a223ce195741ca4f1a0e1a44f3e75ce5662b6c06

Powered by blists - more mailing lists