Message-ID: <20251218150309.00006837@huawei.com>
Date: Thu, 18 Dec 2025 15:03:09 +0000
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>, <edward.cree@....com>,
	<davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<edumazet@...gle.com>, <dave.jiang@...el.com>
Subject: Re: [PATCH v22 11/25] cxl/hdm: Add support for getting region from
 committed decoder

On Thu, 18 Dec 2025 11:52:58 +0000
Alejandro Lucero Palau <alucerop@....com> wrote:

> Hi Jonathan,
> 
> 
> On 12/15/25 13:50, Jonathan Cameron wrote:
> > On Fri, 5 Dec 2025 11:52:34 +0000
> > <alejandro.lucero-palau@....com> wrote:
> >  
> >> From: Alejandro Lucero <alucerop@....com>
> >>
> >> A Type2 device configured by the BIOS can already have its HDM
> >> committed. Add a cxl_get_committed_decoder() function for cheking  
> > checking if this is so after memdev creation.
> >  
> >> so after memdev creation. A CXL region should have been created
> >> during memdev initialization, therefore a Type2 driver can ask for
> >> such a region for working with the HPA. If the HDM is not committed,
> >> a Type2 driver will create the region after obtaining proper HPA
> >> and DPA space.
> >>
> >> Signed-off-by: Alejandro Lucero <alucerop@....com>  
> > Hi Alejandro,
> >
> > I'm in two minds about this.  In general there are devices that have
> > been configured by the BIOS because they are already in use. I'm not sure
> > the driver you are working with here is necessarily set up to survive
> > that sort of live setup without interrupting data flows.  
> 
> 
> This is not mainly about my driver/device but something PJ and Dan agree 
> on support along this type2 patchset.
> 
> You can see the v21 discussions, but basically PJ can not have his 
> driver using the committed decoders from BIOS. So this change addresses 
> that situation which my driver/device can also benefit from as current 
> BIOS available is committing decoders regardless of UEFI flags like 
> EFI_RESERVED_TYPE.
> 
> 
> Neither in my case nor in PJ case the device will be in use before 
> kernel is executing, although PJ should confirm this.

There was some discussion in that thread of whether the decoders are locked.
If they aren't (and if the device is not in use, or some other hard constraint
isn't requiring it, in my view they definitely shouldn't be!) I'd at least
like to consider the option of a 'cleanup pass' to tear them down and give
the driver a clean slate to build on. Kind of similar to what we do in
making PCI reeumerate in the kernel if we really don't like what the bios did.

Might not be possible if there is another higher numbered decoder in use
though :(

> 
> 
> >
> > If it is fair enough to support this, otherwise my inclination is tear
> > down whatever the bios did and start again (unless locked - in which
> > case go grumble at your BIOS folk). Reasoning being that we then only
> > have to handle the equivalent of the hotplug flow in both cases rather
> > than having to handle 2.  
> 
> 
> Well, the automatic discovery region used for Type3 can be reused for 
> Type2 in this scenario, so we do not need to tear down what the BIOS 
> did. However, the argument is what we should do when the driver exits 
> which the current functionality added with the patchset being tearing 
> down the device and CXL bridge decoders. Dan seems to be keen on not 
> doing this tear down even if the HDMs are not locked.

That's the question that makes this interesting.  What is reasoning for
leaving bios stuff around in type 2 cases? I'd definitely like 'a way'
to blow it away even if another option keeps it in place.
A bios configures for what it can see at boot not necessarily what shows
up later.  Similar cases exist in PCI such as resizeable BARs.
The OS knows a lot more about the workload than the bios ever does and
may choose to reconfigure because of hotplugged devices.

> 
> 
> What I can say is I have tested this patchset with an AMD system and 
> with the BIOS committing the HDM decoders for my device, and the first 
> time the driver loads it gets the region from the automatic discovery 
> while creating memdev, and the driver does tear down the HDMs when 
> exiting. Subsequent driver loads do the HDM configuration as this 
> patchset had been doing from day one. So all works as expected.
> 
> 
> I'm inclined to leave the functionality as it is now, and your 
> suggestion or Dan's one for keeping the HDMs, as they were configured by 
> the BIOS, when driver exits should require, IMO, a good reason behind it.

I'd definitely not make the assumption that BIOS' always do things for
good reasons. They do things because someone once thought there was
a good reason - or some other OS relied on them doing some part of setup. 


> 
> 
> > There are also the TSP / encrypted link cases where we need to be careful.
> > I have no idea if that applies here.  
> 
> 
> I would say, let's wait until this support is completed, but as far as I 
> know, this is not a requirement for current Type2 clients (sfc and jump 
> trading).

Dealing with this later works for me.  As long as it fails cleanly all good.

Jonathan

Powered by blists - more mailing lists