linux-kernel - Re: [PATCH v2] cxl: core/region - ignore interleave granularity when ways=1

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: <67ee11cf28f2e_464ec2949a@dwillia2-xfh.jf.intel.com.notmuch>
Date: Wed, 2 Apr 2025 21:42:55 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: "Zhijian Li (Fujitsu)" <lizhijian@...itsu.com>, Gregory Price
	<gourry@...rry.net>, "linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kernel-team@...a.com" <kernel-team@...a.com>, "dan.j.williams@...el.com"
	<dan.j.williams@...el.com>, "vishal.l.verma@...el.com"
	<vishal.l.verma@...el.com>, "dave.jiang@...el.com" <dave.jiang@...el.com>,
	"dave@...olabs.net" <dave@...olabs.net>, "jonathan.cameron@...wei.com"
	<jonathan.cameron@...wei.com>, "alison.schofield@...el.com"
	<alison.schofield@...el.com>, "ira.weiny@...el.com" <ira.weiny@...el.com>,
	"Yasunori Gotou (Fujitsu)" <y-goto@...itsu.com>
Subject: Re: [PATCH v2] cxl: core/region - ignore interleave granularity when
 ways=1

Zhijian Li (Fujitsu) wrote:
> Hi Gregory and CXL community
> Cc Goto-san
> 
> Wow, our platform has encountered a similar issue, and I am intending to consult
> the community regarding this matter.
> 
> I drafted similar patch locally, but I wonder if we should "ignore" the IG
> or "program" the IG to the decoder.
> 
> Let me post the mail(question) from my drafts in your thread(I hope you I hope you won't mind).
> ======================================
> [Question] granularity is a don't care if not interleaving?
> I saw this sentence " granularity is a don't care if not interleaving" in this
> patch "[ndctl PATCH 2/6] cxl/list: Add interleave parameters to decoder listings" [1]
> 
> This reminds me that our platform programed an unmatched interleave_granularity in HDM decoders
> between endpoint and the host-bridge, see below:
> 
>                                CXL  Root
>              CFMW0           /           \      CFMW1
>             decoder0.0                        decoder0.1
>        128 GiB       IW:1  IG:256           IW:1  IG:256      128 GiB
>                               \           /
>                                Host Bridge
>                               /           \
>                      decoder5.0           decoder5.1
>                   IW:1  IG:256            IW:1  IG:256
> 	      /                                  \
>           Endpoint9                           Endpoint10
>                |                                   |
>           decoder9.0                           decoder10.0
>          IW:1 IG:1024                          IW:1 IG:1024

Why 1024? Yes, the value does not matter, but attempting 1024 feels more
like a unit test than a production use case.

> 
> With this setup, the Linux kernel attempts to create regions for Endpoint9 and Endpoint10
> but fails because the endpoint decoders’ interleave granularity (IG=1024) does not
> match their parent decoders’ IG (256). Ideally, the endpoint decoders are expected to be
> configured for IG=256.
> 
> Currently, we learned that we have only special handling for the root decoders [2][3].
> 
> My question are:
> Q1: whether "granularity is a don't care if not interleaving" is applied to
> all HDM decoders(including root decoder and HDM decoder)

All decoders.

> In current cxl cli , it will not show any interleave_granularity at all when ways==1(no-interleaving)
> $ cxl list -PDE | grep granularity  # show nothing when ways==1

Right, because the value theoretically has no functional impact in the
ways==1 case. However, it errantly ends up having practical impact in
these corners cases where code performs granularity comparisons without
considering that ways may be 1.

> Per the CXL Spec r3.1
> IG: "The number of consecutive bytes that are assigned to each target in the Target List."
> Q2: Does this imply a configuration where the number of ways>1?

Right, the granularity is the boundary at which the decoder switches to
the next target in the target list. When ways=1 granularity can be
infinity or zero by that definition.

> Q3: Does the IG also represent the device's capabilities? When programming, should one also
> consider whether the device supports it?

Yes, see bits [9:8] in the CXL HDM Decoder Capability Register (CXL 3.2
8.2.4.20.1). So even though the math should not matter, I would still
expect the driver to try to be careful to make sure that IG+8 is less
than the address-bit max.

> If "granularity is a don't care if not interleaving" is true, how about below changes

Part of me says, "yes, that should be ok", another part of me says "what
is the practical benefit of allowing any granularity to be specified?".

So the fix from Gregory is limited to the case of "whoops, the platform
BIOS thought this was a good idea even though it does not matter in
practice, teach Linux to be lenient in this case.".

The proposal to accept that in all case allows user-created regions to
have odd large granularity sizes in the iw=1 case, and I am skeptical
it is worth supporting that now.