Re: [PATCH v4 09/10] PCI: save/restore CXL config around reset

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Mon, 26 Jan 2026 15:34:35 -0700
Alex Williamson <alex@shazbot.org> wrote:

> On Thu, 22 Jan 2026 10:47:45 +0000
> Jonathan Cameron <jonathan.cameron@huawei.com> wrote:
> 
> > On Thu, 22 Jan 2026 11:01:57 +0100
> > Lukas Wunner <lukas@wunner.de> wrote:
> >   
> > > On Tue, Jan 20, 2026 at 10:26:09PM +0000, smadhavan@nvidia.com wrote:    
> > > > +++ b/drivers/pci/pci.c
> > > > @@ -4989,6 +4990,11 @@ static int cxl_reset(struct pci_dev *dev, bool probe)
> > > >  	if (probe)
> > > >  		return 0;
> > > > 
> > > > +	pci_save_state(dev);
> > > > +	rc = cxl_config_save_state(dev, &cxl_state);
> > > > +	if (rc)
> > > > +		pci_warn(dev, "Failed to save CXL config state: %d\n", rc);
> > > > +      
> > > 
> > > Hm, shouldn't the call to cxl_config_save_state() be moved to
> > > pci_save_state() (and likewise, cxl_config_restore_state() moved to
> > > pci_restore_state())?
> > > 
> > > E.g. when a DPC event occurs, I assume CXL registers need to
> > > be restored as well on recovery, right?    
> > The CXL spec has some comic language around DPC that basically says
> > "use with care, DPC trigger will bring down physical link, reset devicestate,
> > disrupt CXL.cache and CXL.mem traffic".
> > or in shorter words
> > 'Good luck'
> > 
> > If a CXL device undergoes DPC high chance you'll either trigger CXL isolation
> > which we aren't handing yet in Linux because we aren't convinced software
> > can really recover form it, or stall a CPU and end up rebooting.
> > 
> > Maybe we'll one day we'll figure this out. Today turn off DPC on CXL ports! :)  
> 
> Even if we hand-wave that DPC isn't an issue, save/restore of the PCI
> state happens at a higher level for every other PCI reset method and
> we're creating inconsistency here.
> 
> PCI-core includes interfaces for saving PCI state, offloading PCI state
> as an opaque blob, reloading, and restoring that state, and performing
> resets without saving and restoring state.  This has a couple users,
> including vfio.
> 
> If we want similar behavior for CXL type2 devices for a future vfio use
> case, we shouldn't create unnecessary differentiation here with saving
> the CXL state separately and making the reset method behave
> differently.  Thanks,
> 

I'm a bit concerned that, unlike PCI where no traffic flows after reset
and restore of basic PCIe stuff, for CXL once you've put the decoders
etc back in place, CXL.mem traffic can happen autonomously. It's
cacheable and physical address prefetchers on the CPU side may be able
wander into it more or less randomly, whether there are page tables yet
or not.

This is somewhat similar to PCI devices misbehaving if you enable
bus mastering without ensuring they are in a clean state (just in the
other direction).

So I'm not sure how safe it is to restore the generic CXL state with
out the driver taking control.

I don't think there are tight enough guarantees that devices should be
able to survive this if their drivers haven't managed the setup of CXL.mem
carefully as they did during driver bind etc.  Maybe they had to
load a firmware first before there was anything behind a CXL protocol
front end.

The drivers can't stop CXL.mem in a prepare reset callback
prior to saving state as it may be RWL by an annoying BIOS.

Maybe I'm overly paranoid and all device manufacturers are sensible.
Or I missed some spec text that says devices should politely handle
traffic turning up before they are ready.  If they implement the memory
ready checks then we may be fine as hopefully Media Status == Ready
doesn't happen until it's safe to enable access (though the spec
doesn't actually say that is sufficient that I can find).

I need to do some more digging and maybe a spot of prototyping.
Also more than plausible I'm missing a nugget of code in here
that makes this all safe.

Jonathan


> Alex