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: <20240827160625.0000505c@Huawei.com>
Date: Tue, 27 Aug 2024 16:06:25 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Alejandro Lucero Palau <alucerop@....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 Mon, 19 Aug 2024 12:10:34 +0100
Alejandro Lucero Palau <alucerop@....com> wrote:

> On 8/15/24 17:35, Jonathan Cameron wrote:
> > On Mon, 12 Aug 2024 12:16:02 +0100
> > Alejandro Lucero Palau <alucerop@....com> 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 ...  
> > If most code can't see the internals of cxl_dev_state because it
> > doesn't include the header that defines it, then we will generally
> > spot data that may not belong in that state structure in the first place
> > or where it is appropriate to have an accessor function mediating that
> > access.  
> 
> 
> I follow that but I do not know if you are suggesting here to make it 
> opaque which conflicts with a previous comment stating it does not need 
> to be.
> 
Different potential approaches.  I'm not totally sure we 'yet' care
about making it opaque as we don't have that many drivers so review for
misuse is enough. Longer term I think we want to get there - maybe now
is the convenient moment to do so.

Jonathan

> 
> > Jonathan
> >
> >