[PATCH v6 00/14] cxl: Address translation support, part 1: Cleanups and refactoring

Robert Richter posted 14 patches 7 months, 1 week ago
drivers/cxl/acpi.c        |  10 ++-
drivers/cxl/core/cdat.c   |   2 +-
drivers/cxl/core/hdm.c    |   3 +-
drivers/cxl/core/memdev.c |   4 +-
drivers/cxl/core/pci.c    |  48 +++++++----
drivers/cxl/core/port.c   |  23 ++---
drivers/cxl/core/region.c | 177 +++++++++++++++++++++++---------------
drivers/cxl/cxl.h         |  13 +--
drivers/cxl/port.c        |  15 +---
9 files changed, 173 insertions(+), 122 deletions(-)
[PATCH v6 00/14] cxl: Address translation support, part 1: Cleanups and refactoring
Posted by Robert Richter 7 months, 1 week ago
This series is the first part of adding support for CXL address
translation. It contains cleanup and code refactoring in preparation
of the actual implementation that will be sent in part 2. Cleanup and
code refactoring have been split in a separate series to reduce the
number of patches of the series. Even without address translation on
top this rework improves esp. the region code, cleans it up,
simplifies it and adds debugging messages to better analyze region
creation failures:

Content of patches:

 * Patches 1: Remove else after return.

 * Patches 2-3: Cleanups and comments around cxl_hdm_decode_init().

 * Patches 4-6: Adding and modifying helper functions.

 * Patches 7-11: Refactoring of endpoint decoder setup and cxl_find*()
   including cleanup helpers.

 * Patches 12-14: Adding and modifying debug messages.

v6:
 * added tags to SOB chain,
 * added more occurences to remove-else-after-return (Alison),
 * updated description of cxl_port_pick_region_decoder() (Fabio),

v5:
 * added tags to SOB chain,
 * made comment a oneliner in cxl_hdm_decode_init() (Jonathan),
 * updated patch description introducing parent_port_of() (Fabio),
 * removed EXPORT_SYMBOL_NS_GPL() of function parent_port_of() (Dan),
 * renamed functions to cxl_port_pick_region_decoder() and
   cxl_rr_assign_decoder(), updated descriptions (Dan),
 * added patch to replace put_cxl_root() by a cleanup helper,
 * using __free() for reference counting of cxl_find_*() functions,
   added cleanup helpers (Dan),
 * dropped patch adding CFMWS memory log messages (Dan),

v4:
 * rebased onto cxl/next, commit 0a14566be090 ("cxl/Documentation:
   Remove 'mixed' from sysfs mode doc"),
 * added tags to SOB chain,
 * reworked comments in cxl_hdm_decode_init() (dropped moving comment
   and updated patch that modifies comments) (Jonathan),
 * reworded patch description that removes duplicate call of
   cxl_find_decoder_early() (Jonathan),
 * moved some patches out of this rework and cleanup series (Dave,
   Jonathan),

v3:
 * added tags to SOB chain,
 * fixed NULL pointer dereference in cxl_find_root_decoder() (Alison),
 * updated subject line of patches that add kernel messages and
   included example log messages (Alison),

v2:
 * rebased onto cxl/next,
 * added tags to SOB chain,
 * move patches with cleanups and refactoring into this separate
   series (Dave),
 * added patch "cxl/acpi: Unify CFMWS memory log messages with SRAT
   messages" to improve CFMWS log messages,
 * renamed endpoint decoder functions to cxl_endpoint_decoder_*() (Li),
 * reworded patch description that moves find_cxl_root() and reworks
   cxl_find_root_decoder() (Terry),
 * small changes to cxl_find_root_decoder()/
   cxl_endpoint_decoder_initialize() (Jonanthan),
 * updated comment in cxl_port_find_switch_decoder() (Ben),
 * cxl_endpoint_decoder_initialize(): Simplify variable declaration
   (Jonathan, Ben),
 * cxl_find_decoder_early(): Added comment on function usage (Gregory),
 * reordered patches and reworded some of the subject for a better
   structure.

Robert Richter (14):
  cxl: Remove else after return
  cxl/pci: Moving code in cxl_hdm_decode_init()
  cxl/pci: Add comments to cxl_hdm_decode_init()
  cxl: Introduce parent_port_of() helper
  cxl/region: Rename function to cxl_port_pick_region_decoder()
  cxl/region: Avoid duplicate call of cxl_port_pick_region_decoder()
  cxl/region: Move find_cxl_root() to cxl_add_to_region()
  cxl/port: Replace put_cxl_root() by a cleanup helper
  cxl/region: Factor out code to find the root decoder
  cxl/region: Factor out code to find a root decoder's region
  cxl/region: Add function to find a port's switch decoder by range
  cxl/region: Add a dev_warn() on registration failure
  cxl/region: Add a dev_err() on missing target list entries
  cxl: Add a dev_dbg() when a decoder was added to a port

 drivers/cxl/acpi.c        |  10 ++-
 drivers/cxl/core/cdat.c   |   2 +-
 drivers/cxl/core/hdm.c    |   3 +-
 drivers/cxl/core/memdev.c |   4 +-
 drivers/cxl/core/pci.c    |  48 +++++++----
 drivers/cxl/core/port.c   |  23 ++---
 drivers/cxl/core/region.c | 177 +++++++++++++++++++++++---------------
 drivers/cxl/cxl.h         |  13 +--
 drivers/cxl/port.c        |  15 +---
 9 files changed, 173 insertions(+), 122 deletions(-)


base-commit: 8e62ba590160f91abba6490d9c17aa13bada4752
-- 
2.39.5
Re: [PATCH v6 00/14] cxl: Address translation support, part 1: Cleanups and refactoring
Posted by Dan Williams 7 months, 1 week ago
Robert Richter wrote:
> This series is the first part of adding support for CXL address
> translation. It contains cleanup and code refactoring in preparation
> of the actual implementation that will be sent in part 2. Cleanup and
> code refactoring have been split in a separate series to reduce the
> number of patches of the series. Even without address translation on
> top this rework improves esp. the region code, cleans it up,
> simplifies it and adds debugging messages to better analyze region
> creation failures:
> 
[..]
> 
> v6:
>  * added tags to SOB chain,
>  * added more occurences to remove-else-after-return (Alison),
>  * updated description of cxl_port_pick_region_decoder() (Fabio),
[..]

Aside from the "obj __free(...) = NULL" anti-pattern to clean up, for
the series, you can add:

Acked-by: Dan Williams <dan.j.williams@intel.com>
Re: [PATCH v6 00/14] cxl: Address translation support, part 1: Cleanups and refactoring
Posted by Dave Jiang 7 months, 1 week ago

On 5/9/25 8:06 AM, Robert Richter wrote:
> This series is the first part of adding support for CXL address
> translation. It contains cleanup and code refactoring in preparation
> of the actual implementation that will be sent in part 2. Cleanup and
> code refactoring have been split in a separate series to reduce the
> number of patches of the series. Even without address translation on
> top this rework improves esp. the region code, cleans it up,
> simplifies it and adds debugging messages to better analyze region
> creation failures:
> 
> Content of patches:
> 
>  * Patches 1: Remove else after return.
> 
>  * Patches 2-3: Cleanups and comments around cxl_hdm_decode_init().
> 
>  * Patches 4-6: Adding and modifying helper functions.
> 
>  * Patches 7-11: Refactoring of endpoint decoder setup and cxl_find*()
>    including cleanup helpers.
> 
>  * Patches 12-14: Adding and modifying debug messages.
> 
> v6:
>  * added tags to SOB chain,
>  * added more occurences to remove-else-after-return (Alison),
>  * updated description of cxl_port_pick_region_decoder() (Fabio),
> 
> v5:
>  * added tags to SOB chain,
>  * made comment a oneliner in cxl_hdm_decode_init() (Jonathan),
>  * updated patch description introducing parent_port_of() (Fabio),
>  * removed EXPORT_SYMBOL_NS_GPL() of function parent_port_of() (Dan),
>  * renamed functions to cxl_port_pick_region_decoder() and
>    cxl_rr_assign_decoder(), updated descriptions (Dan),
>  * added patch to replace put_cxl_root() by a cleanup helper,
>  * using __free() for reference counting of cxl_find_*() functions,
>    added cleanup helpers (Dan),
>  * dropped patch adding CFMWS memory log messages (Dan),
> 
> v4:
>  * rebased onto cxl/next, commit 0a14566be090 ("cxl/Documentation:
>    Remove 'mixed' from sysfs mode doc"),
>  * added tags to SOB chain,
>  * reworked comments in cxl_hdm_decode_init() (dropped moving comment
>    and updated patch that modifies comments) (Jonathan),
>  * reworded patch description that removes duplicate call of
>    cxl_find_decoder_early() (Jonathan),
>  * moved some patches out of this rework and cleanup series (Dave,
>    Jonathan),
> 
> v3:
>  * added tags to SOB chain,
>  * fixed NULL pointer dereference in cxl_find_root_decoder() (Alison),
>  * updated subject line of patches that add kernel messages and
>    included example log messages (Alison),
> 
> v2:
>  * rebased onto cxl/next,
>  * added tags to SOB chain,
>  * move patches with cleanups and refactoring into this separate
>    series (Dave),
>  * added patch "cxl/acpi: Unify CFMWS memory log messages with SRAT
>    messages" to improve CFMWS log messages,
>  * renamed endpoint decoder functions to cxl_endpoint_decoder_*() (Li),
>  * reworded patch description that moves find_cxl_root() and reworks
>    cxl_find_root_decoder() (Terry),
>  * small changes to cxl_find_root_decoder()/
>    cxl_endpoint_decoder_initialize() (Jonanthan),
>  * updated comment in cxl_port_find_switch_decoder() (Ben),
>  * cxl_endpoint_decoder_initialize(): Simplify variable declaration
>    (Jonathan, Ben),
>  * cxl_find_decoder_early(): Added comment on function usage (Gregory),
>  * reordered patches and reworded some of the subject for a better
>    structure.
> 
> Robert Richter (14):
>   cxl: Remove else after return
>   cxl/pci: Moving code in cxl_hdm_decode_init()
>   cxl/pci: Add comments to cxl_hdm_decode_init()
>   cxl: Introduce parent_port_of() helper
>   cxl/region: Rename function to cxl_port_pick_region_decoder()
>   cxl/region: Avoid duplicate call of cxl_port_pick_region_decoder()
>   cxl/region: Move find_cxl_root() to cxl_add_to_region()
>   cxl/port: Replace put_cxl_root() by a cleanup helper
>   cxl/region: Factor out code to find the root decoder
>   cxl/region: Factor out code to find a root decoder's region
>   cxl/region: Add function to find a port's switch decoder by range
>   cxl/region: Add a dev_warn() on registration failure
>   cxl/region: Add a dev_err() on missing target list entries
>   cxl: Add a dev_dbg() when a decoder was added to a port
> 
>  drivers/cxl/acpi.c        |  10 ++-
>  drivers/cxl/core/cdat.c   |   2 +-
>  drivers/cxl/core/hdm.c    |   3 +-
>  drivers/cxl/core/memdev.c |   4 +-
>  drivers/cxl/core/pci.c    |  48 +++++++----
>  drivers/cxl/core/port.c   |  23 ++---
>  drivers/cxl/core/region.c | 177 +++++++++++++++++++++++---------------
>  drivers/cxl/cxl.h         |  13 +--
>  drivers/cxl/port.c        |  15 +---
>  9 files changed, 173 insertions(+), 122 deletions(-)
> 
> 
> base-commit: 8e62ba590160f91abba6490d9c17aa13bada4752

Applied to cxl/next. Thanks Robert!
Re: [PATCH v6 00/14] cxl: Address translation support, part 1: Cleanups and refactoring
Posted by Dave Jiang 7 months, 1 week ago

On 5/9/25 9:03 AM, Dave Jiang wrote:
> 
> 
> On 5/9/25 8:06 AM, Robert Richter wrote:
>> This series is the first part of adding support for CXL address
>> translation. It contains cleanup and code refactoring in preparation
>> of the actual implementation that will be sent in part 2. Cleanup and
>> code refactoring have been split in a separate series to reduce the
>> number of patches of the series. Even without address translation on
>> top this rework improves esp. the region code, cleans it up,
>> simplifies it and adds debugging messages to better analyze region
>> creation failures:
>>
>> Content of patches:
>>
>>  * Patches 1: Remove else after return.
>>
>>  * Patches 2-3: Cleanups and comments around cxl_hdm_decode_init().
>>
>>  * Patches 4-6: Adding and modifying helper functions.
>>
>>  * Patches 7-11: Refactoring of endpoint decoder setup and cxl_find*()
>>    including cleanup helpers.
>>
>>  * Patches 12-14: Adding and modifying debug messages.
>>
>> v6:
>>  * added tags to SOB chain,
>>  * added more occurences to remove-else-after-return (Alison),
>>  * updated description of cxl_port_pick_region_decoder() (Fabio),
>>
>> v5:
>>  * added tags to SOB chain,
>>  * made comment a oneliner in cxl_hdm_decode_init() (Jonathan),
>>  * updated patch description introducing parent_port_of() (Fabio),
>>  * removed EXPORT_SYMBOL_NS_GPL() of function parent_port_of() (Dan),
>>  * renamed functions to cxl_port_pick_region_decoder() and
>>    cxl_rr_assign_decoder(), updated descriptions (Dan),
>>  * added patch to replace put_cxl_root() by a cleanup helper,
>>  * using __free() for reference counting of cxl_find_*() functions,
>>    added cleanup helpers (Dan),
>>  * dropped patch adding CFMWS memory log messages (Dan),
>>
>> v4:
>>  * rebased onto cxl/next, commit 0a14566be090 ("cxl/Documentation:
>>    Remove 'mixed' from sysfs mode doc"),
>>  * added tags to SOB chain,
>>  * reworked comments in cxl_hdm_decode_init() (dropped moving comment
>>    and updated patch that modifies comments) (Jonathan),
>>  * reworded patch description that removes duplicate call of
>>    cxl_find_decoder_early() (Jonathan),
>>  * moved some patches out of this rework and cleanup series (Dave,
>>    Jonathan),
>>
>> v3:
>>  * added tags to SOB chain,
>>  * fixed NULL pointer dereference in cxl_find_root_decoder() (Alison),
>>  * updated subject line of patches that add kernel messages and
>>    included example log messages (Alison),
>>
>> v2:
>>  * rebased onto cxl/next,
>>  * added tags to SOB chain,
>>  * move patches with cleanups and refactoring into this separate
>>    series (Dave),
>>  * added patch "cxl/acpi: Unify CFMWS memory log messages with SRAT
>>    messages" to improve CFMWS log messages,
>>  * renamed endpoint decoder functions to cxl_endpoint_decoder_*() (Li),
>>  * reworded patch description that moves find_cxl_root() and reworks
>>    cxl_find_root_decoder() (Terry),
>>  * small changes to cxl_find_root_decoder()/
>>    cxl_endpoint_decoder_initialize() (Jonanthan),
>>  * updated comment in cxl_port_find_switch_decoder() (Ben),
>>  * cxl_endpoint_decoder_initialize(): Simplify variable declaration
>>    (Jonathan, Ben),
>>  * cxl_find_decoder_early(): Added comment on function usage (Gregory),
>>  * reordered patches and reworded some of the subject for a better
>>    structure.
>>
>> Robert Richter (14):
>>   cxl: Remove else after return
>>   cxl/pci: Moving code in cxl_hdm_decode_init()
>>   cxl/pci: Add comments to cxl_hdm_decode_init()
>>   cxl: Introduce parent_port_of() helper
>>   cxl/region: Rename function to cxl_port_pick_region_decoder()
>>   cxl/region: Avoid duplicate call of cxl_port_pick_region_decoder()
>>   cxl/region: Move find_cxl_root() to cxl_add_to_region()
>>   cxl/port: Replace put_cxl_root() by a cleanup helper
>>   cxl/region: Factor out code to find the root decoder
>>   cxl/region: Factor out code to find a root decoder's region
>>   cxl/region: Add function to find a port's switch decoder by range
>>   cxl/region: Add a dev_warn() on registration failure
>>   cxl/region: Add a dev_err() on missing target list entries
>>   cxl: Add a dev_dbg() when a decoder was added to a port
>>
>>  drivers/cxl/acpi.c        |  10 ++-
>>  drivers/cxl/core/cdat.c   |   2 +-
>>  drivers/cxl/core/hdm.c    |   3 +-
>>  drivers/cxl/core/memdev.c |   4 +-
>>  drivers/cxl/core/pci.c    |  48 +++++++----
>>  drivers/cxl/core/port.c   |  23 ++---
>>  drivers/cxl/core/region.c | 177 +++++++++++++++++++++++---------------
>>  drivers/cxl/cxl.h         |  13 +--
>>  drivers/cxl/port.c        |  15 +---
>>  9 files changed, 173 insertions(+), 122 deletions(-)
>>
>>
>> base-commit: 8e62ba590160f91abba6490d9c17aa13bada4752
> 
> Applied to cxl/next. Thanks Robert!
> 
> 
Robert,
I applied Dan's requested changes and also picked up his ACK's. Please check cxl/next to make sure everything looks good to you. Thanks!
Re: [PATCH v6 00/14] cxl: Address translation support, part 1: Cleanups and refactoring
Posted by Robert Richter 7 months, 1 week ago
On 09.05.25 10:11:43, Dave Jiang wrote:
> On 5/9/25 9:03 AM, Dave Jiang wrote:

> > Applied to cxl/next. Thanks Robert!
> > 
> > 
> Robert,
> I applied Dan's requested changes and also picked up his
> ACK's. Please check cxl/next to make sure everything looks good to
> you. Thanks!

Thank you all for your reviews.

-Robert