netdev - Re: [PATCH v16 04/22] cxl: Move register/capability check to driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e60307b8-f865-4e53-9ea6-13e198eae24d@amd.com>
Date: Fri, 23 May 2025 10:12:01 +0100
From: Alejandro Lucero Palau <alucerop@....com>
To: Dan Williams <dan.j.williams@...el.com>, alejandro.lucero-palau@....com,
 linux-cxl@...r.kernel.org, netdev@...r.kernel.org, edward.cree@....com,
 davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
 edumazet@...gle.com, dave.jiang@...el.com
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v16 04/22] cxl: Move register/capability check to driver

On 5/22/25 20:51, Dan Williams wrote:
> Alejandro Lucero Palau wrote:
> [..]
>> You did not like to add a new capability field to current structs
>> because the information needed was already there in the map. I think it
>> was a fair comment, so the caps, a variable the caller gives, is set
>> during the discovery without any internal struct added.
> The objection was not limited to data structure changes, it also
> includes sprinkling an @caps argument throughout the stack for an
> as yet to be determined benefit.
>
>> Regarding what you suggest below, I have to disagree. This change was
>> introduced for dealing with a driver using CXL, that is a Type2 or
>> future Type1 driver. IMO, most of the innerworkings should be hidden to
>> those clients and therefore working with the map struct is far from
>> ideal, and it is not currently accessible from those drivers.
> Checking a couple validity flags in a now public (in include/cxl/pci.h)
> data-structure is far from ideal?
>
>> With these new drivers the core does not know what should be there, so
>> the check is delayed and left to the driver.
> Correct, left to the driver to read from an existing mechanism.
>
>> IMO, from a Type2/Type1 driver perspective, it is better to deal with
>> caps expected/found than being aware of those internal CXL register
>> discovery and maps.
> Not if a maintainer of the CXL register discovery and maps remains
> unconvinced to merge a parallel redundant mechanism to achieve the exact
> same goal.

OK. You are the maintainer and you'll get what you want. I'm not going 
to fight this if none else back me up.

Because you refer to your maintainer position, let me just say, in a 
critical but constructive way, I'm a bit pissed off with this.

It is not because we disagree nor because you as the maintainer have a 
weight on this I don't. I accept that and I am also happy to discuss all 
this with you even if I end up doing the things your way. I'm pissed off 
because you have been silent during months, with other people in the CXL 
kernel community reviewing the patches, commenting and raising concerns. 
I think it is discouraging that you, as the maintainer, allow me and 
these people involved in those reviews, going through a path you 
disagree with and say nothing. Again, it is not because you have another 
view, surely more relevant than those less used to the code involved, 
but because you disappear for so long.

I need some days off now, what is well-aligned with a four-day long 
weekend for me, but I'll be back with new energy next week for 
addressing all those concerns.

Thank you