Re: [PATCH v17 00/22] Type2 device basic support

[Alejandro Lucero Palau <alucerop@amd.com>]

On 9/4/25 18:48, PJ Waskiewicz wrote:
> Hi Alejandro,
>
> Apologies for the late reply.  Totally lost the reply during the US
> holiday...


No Worries!


> 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@amd.com
>>> wrote:
>>>
>>> Hi Alejandro,
>>>
>>>> From: Alejandro Lucero <alucerop@amd.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.


Just for being sure, you are mmaping the CXL range assigned by the BIOS 
to your device. Right?


>
> 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


It is hard to follow these calls when using v16. But your next email is, 
I think, more interesting. Anyways, if you can use v17, it could help 
for focusing on a specific version.


>
> 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.
>

No, there is no fix in v17.


About lspci output, as you can not put it all, could you tell what is 
the CXL range assigned to the CXL Root bridge and the range assigned to 
your device in those systems you did try? I'm saying so because I think 
you will run into another problem later on with AMD systems, once you 
hopefully solve the one making the current initialization to fail.


Thanks


>>> 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