Message-ID: <44898314e457669a80ccb08976813161d8cd9eb1.camel@kernel.org>
Date: Thu, 04 Sep 2025 10:48:27 -0700
From: PJ Waskiewicz <ppwaskie@...nel.org>
To: Alejandro Lucero Palau <alucerop@....com>,
 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, dave.jiang@...el.com
Subject: Re: [PATCH v17 00/22] Type2 device basic support

Hi Alejandro,

Apologies for the late reply.  Totally lost the reply during the US
holiday...

On Thu, 2025-08-28 at 09:02 +0100, Alejandro Lucero Palau wrote:
> Hi PJ,
> 
> On 8/27/25 17:48, PJ Waskiewicz wrote:
> > On Tue, 2025-06-24 at 15:13 +0100, alejandro.lucero-palau@....com
> > wrote:
> > 
> > Hi Alejandro,
> > 
> > > From: Alejandro Lucero <alucerop@....com>
> > > 
> > > v17 changes: (Dan Williams review)
> > >   - use devm for cxl_dev_state allocation
> > >   - using current cxl struct for checking capability registers
> > > found
> > > by
> > >     the driver.
> > >   - simplify dpa initialization without a mailbox not supporting
> > > pmem
> > >   - add cxl_acquire_endpoint for protection during initialization
> > >   - add callback/action to cxl_create_region for a driver
> > > notified
> > > about cxl
> > >     core kernel modules removal.
> > >   - add sfc function to disable CXL-based PIO buffers if such a
> > > callback
> > >     is invoked.
> > >   - Always manage a Type2 created region as private not allowing
> > > DAX.
> > > 
> > I've been following the patches here since your initial RFC.  What
> > platform are you testing these on out of curiosity?
> 
> 
> Most of the work was done with qemu. Nowadays, I have several system 
> with CXL support and Type2 BIOS support, so it has been successfully 
> tested there as well.

I also have a number of systems with Type2 support enabled in the BIOS,
spread between multiple uarch versions of Intel and AMD (EMR/GNR,
Genoa/Turin).

> 
> > I've tried pulling the v16 patches into my test environment, and on
> > CXL
> > 2.0 hosts that I have access to, the patches did not work when
> > trying
> > to hook up a Type 2 device.  Most of it centered around many of the
> > CXL
> > host registers you try poking not existing.
> 
> 
> Can you share the system logs and maybe run it with CXL debugging on?

What system logs are you referring to?  dmesg?  Also what CXL
debugging?  Just enabling the dev_dbg() paths for the CXL modules?

> 
> > I do have CXL-capable BIOS
> > firmware on these hosts, but I'm questioning that either there's
> > still
> > missing firmware, or the patches are trying to touch something that
> > doesn't exist.
> 
> 
> May I ask which system are you using? ARM/Intel/AMD/surpriseme? lspci
> -vvv output would also be useful. I did find some issues with how the
> BIOS we got is doing things, something I will share and work on if
> that 
> turns out to be a valid case and not a BIOS problem.

I've been lately testing on an Intel GNR and an AMD Turin.  Let's just
say we can focus on the CRB's from both of them, so I have BIOS's
directly from the CPU vendors (there are other OEM vendors in the mix,
same results, but we'll leave them out for now).

We have our Type2 device that successfully links/trains CXL protocols
(all of them), and have been working for some time on previous gen's as
well (SPR/EMR/Genoa).  I can't share the full output of lspci due to
this being a proprietary device, but link caps show the .mem and other
protocols fully linked/trained.  I also have the .mem acceleration
region mapped currently by our drivers directly.

What I'm running into is very early in the driver bringup when
migrating to the new API you have presented with the refactors of the
CXL core.  In my driver's .probe() function (assume this is a pci_dev),
I have the following beginning flow:

- pci_find_dvsec_capability() (returns the correct field pointer)
- cxl_dev_state_create(..., CXL_DEVTYPE_DEVMEM, ...) - succeeds
- cxl_pci_accel_setup_regs() - fails to detect accelerated registers
- cxl_mem_dpa_init()
- cxl_dpa_setup() - returns failure

This is where the wheels have already flown off.  Note that this is
with the V16 patches, so I'm not sure if there was something resolved
between those and the V17 patches.  I'm working right now on geting the
V17 patches running on my Purico Turin box.  But if there's a specific
BIOS I would need to target for the Purico CRB, that would be useful
information to have as well.  My Purico box is running BIOS Revision
5.33.

> 
> > 
> > I'm working on rebasing to the v17 patches to see if this resolves
> > what
> > I'm seeing.  But it's a bit of a lift, so I figured I'd ask what
> > you're
> > testing on before burning more time.
> > 
> > Eventually I'd like to either give a Tested-by or shoot back some
> > amended patches based on testing.  But I've not been able to get
> > that
> > far yet...
> 
> 
> That would be really good. Let's see if we can figure out what is the
> problem there.

Sounds like a plan to me.  Thanks for doing the heavy lifting here on
these patches.

Cheers,
-PJ

Powered by blists - more mailing lists