netdev - Re: [PATCH v2 01/15] cxl: add type2 device basic support

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: <942da248-7140-6cb0-d960-34e300544d5a@amd.com>
Date: Tue, 20 Aug 2024 11:44:52 +0100
From: Alejandro Lucero Palau <alucerop@....com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: alejandro.lucero-palau@....com, linux-cxl@...r.kernel.org,
 netdev@...r.kernel.org, dan.j.williams@...el.com, martin.habets@...inx.com,
 edward.cree@....com, davem@...emloft.net, kuba@...nel.org,
 pabeni@...hat.com, edumazet@...gle.com, richard.hughes@....com
Subject: Re: [PATCH v2 01/15] cxl: add type2 device basic support


On 8/19/24 12:12, Alejandro Lucero Palau wrote:
>
> On 8/15/24 17:38, Jonathan Cameron wrote:
>> On Tue, 13 Aug 2024 09:30:08 +0100
>> Alejandro Lucero Palau <alucerop@....com> wrote:
>>
>>> On 8/12/24 12:16, Alejandro Lucero Palau wrote:
>>>> On 8/4/24 18:10, Jonathan Cameron wrote:
>>>>> On Mon, 15 Jul 2024 18:28:21 +0100
>>>>> <alejandro.lucero-palau@....com> wrote:
>>>>>> From: Alejandro Lucero <alucerop@....com>
>>>>>>
>>>>>> Differientiate Type3, aka memory expanders, from Type2, aka device
>>>>>> accelerators, with a new function for initializing cxl_dev_state.
>>>>>>
>>>>>> Create opaque struct to be used by accelerators relying on new 
>>>>>> access
>>>>>> functions in following patches.
>>>>>>
>>>>>> Add SFC ethernet network driver as the client.
>>>>>>
>>>>>> Based on
>>>>>> https://lore.kernel.org/linux-cxl/168592149709.1948938.8663425987110396027.stgit@dwillia2-xfh.jf.intel.com/T/#m52543f85d0e41ff7b3063fdb9caa7e845b446d0e 
>>>>>>
>>>>>>
>>>>>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>>>>>> Co-developed-by: Dan Williams <dan.j.williams@...el.com>
>>>>>> +
>>>>>> +void cxl_accel_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec)
>>>>>> +{
>>>>>> +    cxlds->cxl_dvsec = dvsec;
>>>>> Nothing to do with accel. If these make sense promote to cxl
>>>>> core and a linux/cxl/ header.  Also we may want the type3 driver to
>>>>> switch to them long term. If nothing else, making that handle the
>>>>> cxl_dev_state as more opaque will show up what is still directly
>>>>> accessed and may need to be wrapped up for a future accelerator 
>>>>> driver
>>>>> to use.
>>>> I will change the function name then, but not sure I follow the
>>>> comment about more opaque ...
>>>>
>>>>
>>> I have second thoughts about this.
>>>
>>>
>>> I consider this as an accessor  for, as you said in a previous 
>>> exchange,
>>> facilitating changes to the core structs without touching those accel
>>> drivers using it.
>>>
>>> Type3 driver is part of the CXL core and easy to change for these kind
>>> of updates since it will only be one driver supporting all Type3, 
>>> and an
>>> accessor is not required then.
>>>
>>> Let me know what you think.
>> It's less critical, but longer term I'd expect any stuff that makes
>> sense for accelerators and the type 3 driver to use the same
>> approaches and code paths.  Makes it easier to see where they
>> are related than opencoding the accesses in the type 3 driver will
>> do.  In the very long term, I'd expect the type 3 driver to just be
>> another CXL driver alongside many others.
>
>
> It makes sense, so I will change the name.
>
> A following patchset when this is hopefully going through will be to 
> use the accessors in the CXL PCI driver.
>
> Thanks!
>

I realize you likely mean all the accessors and not just the dvsec one. 
Right?

Also, I think I could add the changes to the pci driver for using them 
within this patchset.


>
>> Jonathan
>>
>>>