Re: [PATCH 12/17] cxl/core: Add CXL snoop filter setup and allocation

["Cheatham, Benjamin" <benjamin.cheatham@amd.com>]

On 12/17/2025 10:35 AM, Jonathan Cameron wrote:
> On Tue, 11 Nov 2025 15:40:27 -0600
> Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> 
>> The CXL snoop filter capability (CXL 3.2 8.2.4.23) is a mandatory
>> capability for CXL 2.0+ root ports that is used by the host and CXL
>> devices to manage CXL cache coherence.
>>
>> Snoop filters may be shared by several CXL root ports, with each snoop
>> filter having a unique group id. Create snoop filter instances as CXL
>> root ports are added to the system. If the snoop filter already exists,
>> add the root port to the existing filter instead.
>>
>> Snoop filter instances are used to keep track of the amount of CXL cache
>> space active under the filter. The CXL specification allows rejecting
>> CXL devices based on the amount of filter subscription, but determining
>> when the filter is oversaturated is most likely implementation specific.
>> To address this, add a cxl_core module parameter to specify whether
>> adding a CXL.cache device's cache to an already full filter should fail
>> device probe. The default setting of the parameter is to allow
>> oversaturation of the filter ("strict_snoop=off").
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> ---
>>
>> Jonathan asked about the allocation policy when the snoop filter is
>> "full". I had the device fail probe beforehand, but it got me thinking
>> that it may be worthwhile to add an option to over commit the filter
>> instead, hence the module parameter. I'm game to other suggestions, but
>> a config symbol didn't feel right since the setting probably varies by
>> platform/user.
> 
> Doesn't feel like a module parameter is appropriate as may depend on
> specific devices as well. Ideally we want the capacity and device requirements
> to be exposed to user space before deciding to bring it in and how to handle
> minimum vs ideal levels.
> 
> Tricky topic and once we get a little nearer merging this + have some
> device drivers to poke at lets revisit. Maybe on one of the sync calls.
> 

That makes sense, I just wanted to add something to get the ball rolling. I'm
fine delaying it until this starts being considered for a merge and/or
discussing on a sync call.

>  
>>
>> Also I'll add Cache_SF_Coverage support when this series goes out of
>> RFC (I forgot to do it this time around).
>>
> 
> On a related topic, how do we know the right Cache_SF_Granularity for
> a given system?  Might be a specification gap (or I just can't find it)
> 

That's a good question. I guess I just assumed that it would be programmed
by the platform firmware, but it may be that the OS is supposed to program it?
It's pretty ambiguous, so a spec clarification may be needed.

> Jonathan
> 
>