Re: [PATCH v14 10/16] cxl: Cache endpoint HDM state during PCI enumeration

[Srirangan Madhavan <smadhavan@nvidia.com>]

On 10/7/26 12:37 PM, Alison Schofield wrote:
>> In general I'm noticing a few functions that takes a 'struct pci_dev' that does not need that in resource.c. In the future, I would like to extend cxl_test to this core code in order to cover most of the functions in resource.c and the reset mechanism. Do you mind taking a look at if things can be reorganized? I did a quick refactor of your series using LLM [1]. See if the shaping of that is acceptable for you. Essentially I had it organize into 3 parts. drivers/cxl/core/hdm_regs.c, drivers/cxl/core/hdm_state.c, and drivers/pci/cxl.c. Feel free to use any or none of the changes.
> 
> Srirangan,
> 
> Agree with Dave on this point. We should avoid carrying struct  pci_dev down
> into helpers that don't otherwise need to know about PCI.
> 
> There are a few more examples later in the series where this starts to spread
> through the HDM/range handling. I'll call those out on patch 12/16.
> 
> -- Alison
> 
> 
>> ┌─────┬──────────────────┬───────────────────────────────────────────────┐
>> │  #  │      Patch       │                    Change                     │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │ 1-4 │ unchanged        │ Same commit objects, all Reviewed-by tags     │
>> │     │                  │ kept                                          │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Introduce        │ target_or_skip_reg_val replaced by an         │
>> │ 5   │ reusable HDM     │ endpoint-only settings struct with skip;      │
>> │     │ decoder settings │ switch commit takes the target list directly; │
>> │     │                  │  endpoints write the DPA Skip registers again │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Move HDM decoder │ New built-in hdm_regs.c, built with           │
>> │ 6   │  helpers to      │ obj-$(subst m,y,$(CONFIG_CXL_BUS)), so        │
>> │     │ built-in code    │ linking cxl_core no longer depends on         │
>> │     │                  │ CONFIG_CXL_RESET                              │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Share HDM        │ Unpacks the common config only; Target List   │
>> │ 7   │ decoder register │ and DPA Skip are read where they're used      │
>> │     │  unpacking       │ again                                         │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Reject           │                                               │
>> │ 8   │ overflowing HDM  │ Rebased only                                  │
>> │     │ decoder ranges   │                                               │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Refresh cached   │ Endpoint decoders only; CONFIG_CXL_RESET      │
>> │ 9   │ PCI HDM decoder  │ introduced here; two bug fixes (below)        │
>> │     │ settings         │                                               │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Cache endpoint   │ PCI-side code moved to the new                │
>> │ 10  │ HDM state during │ drivers/pci/cxl.c; cache and cxl_rwsem in the │
>> │     │  enumeration     │  new built-in hdm_state.c                     │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │ 11  │ Add CXL Device   │ Moved to drivers/pci/cxl.c, otherwise         │
>> │     │ Reset sequencing │ identical                                     │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Validate and     │                                               │
>> │ 12  │ synchronize HDM  │ Region-quiesce API in hdm_state.c, so the PCI │
>> │     │ ranges around    │  core never takes cxl_rwsem directly          │
>> │     │ reset            │                                               │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │ 13, │ Reject unsafe    │                                               │
>> │  15 │ scope / Expose   │ Rebased only                                  │
>> │     │ CXL Reset        │                                               │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Restore CXL      │ Decoder restore split into hdm_regs.c, which  │
>> │ 14  │ state after PCI  │ needs no pci_dev                              │
>> │     │ reset            │                                               │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>> │     │ Restore CXL      │                                               │
>> │ 16  │ state after CXL  │ Race fix. Found through testing.              │
>> │     │ bus reset        │                                               │
>> ├─────┼──────────────────┼───────────────────────────────────────────────┤
>>
>> [1]:https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/? 
>> h=cxl-type2-reset

Hi Alison, Dave,

I've adopted the structure from Dave's reference branch in v15:

PCI discovery and reset sequencing are in drivers/pci/cxl.c. Generic HDM 
register and state helpers are in hdm_regs.c and hdm_state.c.

v15 series also incorporates the other pieces like cache fixes and 
decoder refresh during bus-reset restoration from Dave's branch.

Thank you for the refactor.

-- 
Regards,
Srirangan