netdev - Re: [PATCH v21 08/23] cxl/sfc: Map cxl component regs

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: <5ceb8547df95c86d812744b07b0d8152f0503ed3.camel@kernel.org>
Date: Fri, 21 Nov 2025 17:11:49 -0800
From: PJ Waskiewicz <ppwaskie@...nel.org>
To: Alejandro Lucero Palau <alucerop@....com>,
 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
Cc: Edward Cree <ecree.xilinx@...il.com>, Jonathan Cameron
	 <Jonathan.Cameron@...wei.com>, Ben Cheatham <benjamin.cheatham@....com>
Subject: Re: [PATCH v21 08/23] cxl/sfc: Map cxl component regs

On Fri, 2025-11-21 at 11:01 +0000, Alejandro Lucero Palau wrote:
> 
> On 11/21/25 06:54, PJ Waskiewicz wrote:
> > On Wed, 2025-11-19 at 19:22 +0000, alejandro.lucero-palau@....com
> > wrote:
> > 
> > Hi Alejandro,
> 
> 
> Hi PJ,
> 
> 
> <snip>
> 
> 
> > > +	}
> > > +
> > > +	rc = cxl_map_component_regs(&cxl->cxlds.reg_map,
> > > +				    &cxl->cxlds.regs.component,
> > > +				    BIT(CXL_CM_CAP_CAP_ID_RAS));
> > I'm going to reiterate a previous concern here with this.  When all
> > of
> > this was in the CXL core, the CXL core owned whatever BAR these
> > registers were in in its entirety.  Now with a Type2 device,
> > splitting
> > this out has implications.
> 
> 
> I have not forgotten your concern and as I said then, I will work on
> a 
> follow-up for this once basic Type2 support patchset goes through.
> 
> The client linked to this patchset is the sfc driver and we do not
> have 
> this problem for the card supporting CXL. But I fully understand this
> is 
> a problem for other more than potential Type2 API clients.
> 
> 
> > The cxl_map_component_regs() is going to try and map the register
> > map
> > you request as a reserved resource, which will fail if this Type2
> > driver has the BAR mapped (which basically all of these drivers
> > do).
> > 
> > I think it's worth either a big comment or something explicit in
> > the
> > patch description that calls this limitation or restriction out.
> > Hardware designers will be caught off-guard if they design their
> > hardware where the CXL component regs are in a BAR shared by other
> > register maps in their devices.  If they land the CXL regs in the
> > middle of that BAR, they will have to do some serious gymnastics in
> > the
> > drivers to map pieces of their BAR to allow the kernel to map the
> > component regs.  OR...they can have some breadcrumbs to try and
> > design
> > the HW where the CXL component regs are at the very beginning or
> > very
> > end of their BAR.  That way drivers have an easier way to reserve a
> > subset of a contiguous BAR, and allow the kernel to grab the
> > remainder
> > for CXL access and management.
> 
> 
> I have thought about the proper solution for this and IMO implies to
> add 
> a new argument where the client can specify the already mapped memory
> for getting the CXL regs available to the CXL core. It should not be
> too 
> much complicated, but I prefer to leave it for a follow up. Not sure
> if 
> you want something more complicated where the code can solve this 
> without the driver's write awareness, but the call failing could be
> more 
> chatty about this possibility so the user can know.

That would be a good addition.  Maybe something to indicate "hey, go
check if someone else already claimed ownership of this memory region"
instead of using a divining rod to find this in /proc/iomem on a hunch.
:)

> 
> 
> But I agree the current patchset should at least specifically comment
> on 
> this in the code. I will do so in v22, but if there exists generic 
> concern about this case not being supported in the current work, I'll
> be 
> addressing this for such a next patchset version.

If you could capture this in either a comment or just the patch
description, I feel like there's enough paper-trail for people doing
this sort of design work should be informed.

I'd just hate to see all this work you're doing to make it in, and a
hardware designer somewhere not knowing the restrictions, and getting
irritated when their shiny new chip doesn't work with your code.  We
can at least help them with documentation.

Cheers,
-PJ