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

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Oct 06, 2026 at 08:39:37AM -0700, Dave Jiang wrote:
> 
> 
> On 10/1/26 2:22 AM, Srirangan Madhavan wrote:
> > PCI capability initialization runs before BAR resources are finalized,
> > while driver binding is too late for driver-independent reset support.
> > Create pci_dev->hdm during pci_bus_add_device(), after PCI resource setup
> > and before driver binding.
> > 
> > Cache BAR-relative HDM location, global control, decoder settings, and CXL
> > Device DVSEC Control, then publish the completed cache under cxl_rwsem.dpa.
> > Restore PCI_COMMAND after temporary MMIO access and reject decoder-count
> > changes.
> > 
> > Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> > ---
> >  drivers/cxl/core/Makefile   |   3 +-
> >  drivers/cxl/core/pci.c      |  15 +-
> >  drivers/cxl/core/regs.c     |   9 ++
> >  drivers/cxl/core/resource.c | 277 ++++++++++++++++++++++++++++++++++++
> >  drivers/pci/bus.c           |   2 +
> >  drivers/pci/probe.c         |   2 +
> >  include/cxl/cxl.h           |  21 +++
> >  tools/testing/cxl/Kbuild    |   1 -
> >  8 files changed, 326 insertions(+), 4 deletions(-)
> > 

snip

> > diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> > index 2be1df80fcc9..e80500f457a9 100644
> > --- a/tools/testing/cxl/Kbuild
> > +++ b/tools/testing/cxl/Kbuild
> > @@ -55,7 +55,6 @@ obj-m += cxl_core.o
> >  
> >  cxl_core-y := $(CXL_CORE_SRC)/port.o
> >  cxl_core-y += $(CXL_CORE_SRC)/pmem.o
> > -cxl_core-y += $(CXL_CORE_SRC)/regs.o
> >  cxl_core-y += $(CXL_CORE_SRC)/memdev.o
> >  cxl_core-y += $(CXL_CORE_SRC)/mbox.o
> >  cxl_core-y += $(CXL_CORE_SRC)/pci.o
> 
> 
> 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