Message-ID: <ada17fb1-0935-461d-bb60-05beaeb2d0fe@amd.com>
Date: Sat, 29 Nov 2025 16:26:13 +0000
From: Alejandro Lucero Palau <alucerop@....com>
To: PJ Waskiewicz <ppwaskie@...nel.org>, 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 v21 00/23] Type2 device basic support


On 11/28/25 20:29, Alejandro Lucero Palau wrote:
>
> On 11/28/25 19:44, PJ Waskiewicz wrote:
>> Hi Alejandro,
>>
>> On Wed, 2025-11-19 at 19:22 +0000, alejandro.lucero-palau@....com
>> wrote:
>>> From: Alejandro Lucero <alucerop@....com>
>>>
>>> The patchset should be applied on the described base commit then
>>> applying
>>> Terry's v13 about CXL error handling. The first 4 patches come from
>>> Dan's
>>> for-6.18/cxl-probe-order branch with minor modifications.
>>>
>>> v21 changes;
>>>
>>>    patch1-2: v20 patch1 splitted up doing the code move in the second
>>>         patch in v21. (Jonathan)
>>>      patch1-4: adding my Signed-off tag along with Dan's
>>>
>>>    patch5: fix duplication of CXL_NR_PARTITION definition
>>>
>>>    patch7: dropped the cxl test fixes removing unused function. It was
>>>       sent independently ahead of this version.
>>>
>>>    patch12: optimization for max free space calculation (Jonathan)
>>>
>>>    patch19: optimization for returning on error (Jonathan)
>>>
>> So I'm unable to get these patches working with a Type2 device that
>> just needs its existing resources auto-discovered by the CXL core.
>> These patches are assuming the underlying device will require full
>> setup and allocations for DPA and HPA.  I'd argue that a true Type2
>> device will not be doing that today with existing BIOS implementations.
>
>
> Well, I'd argue this patchset is what the sfc driver needs, which is 
> the client for this "initial Type2 basic support".
>
>
>> I've tested this behavior on both Intel and AMD platforms (GNR and
>> Turin), and they're behaving the same way.  Both will train up the
>> Type2 device, see there's an advertised CXL.mem region marked EFI
>> Special Purpose memory, and will map it and program the decoders.
>> These patches partially see those decoders are already programmed, but
>> does not bypass that fact, and still attemps to dynamically allocate,
>> configure, and commit, the whole flow.  This assumption fails the init
>> path.
>
>
> Fair enough. We knew about this and as I said, something I would 
> prefer to do as a follow up work or this patchset will be delayed, 
> likely until a new requirement is found out like the problem about 
> DVSEC BAR already being mapped, then waiting for the next thing not 
> covered in this "initial Type2 basic support".
>
>
>>
>> I think there needs to be a bit of a re-think here.  I briefly chatted
>> with Dan offline about this, and we do think a different approach is
>> likely needed.  The current CXL core for Type3 devices can handle when
>> the BIOS/platform firmware already discovers and maps resources, so we
>> should be able to do that for this case.
>
>
> I'm sad to hear that ... I'm getting internal pressure for getting 
> this Type2 done and I realize now it will require "a different 
> approach" for being accepted.
>
>
> Being honest, this is quite demoralizing. Maybe I'm not the right 
> person to get this through.
>
>

Feeling more positive today ...


Looking at my hack for solving this problem, what I suffered (as I did 
explain) with my testing with a BIOS not supporting yet the 
EFI_RESERVED_TYPE and the EFI_ADAPTER_INFO_PROTOCOL  protocol, I think 
it is possible to include the changes with some minor adjustments and 
without too much code. It relies on the same code than for Type3 when 
initializing the endpoint decoder, so the region will be created 
automatically at that point, although through the device creation + 
probe for the port and the region, so the way for the type2 driver to 
get the HPA to work with (iorenmap) needs to be based on other means 
since the region could not be there after the call for creation the memdev.


It brings other things to discuss about what the type2 should do on 
exit, since the endpoint HDM and the CXL Host Bridge HDM should not be 
modified then when doing the unwinding. So the sooner we can see what 
could be done the better for starting such discussion.


I will send v22 including this functionality early next week. Benjamin 
Cheatham solved this problem with a different approach, what, IMO, is 
more complex, mainly due to the region creation when endpoint decoder is 
initialised precluded by a check, which interestingly Ben proposed in 
earlier patchset versions ...


>
>> If you're going to be at Plumbers in a week or so, this would be a
>> great topic if we could grab a whiteboard somewhere and just hack on
>> it.  Otherwise we can also chat on the Discord (I just joined finally)
>>
>> Cheers,
>> -PJ

Powered by blists - more mailing lists