linux-kernel - Re: [PATCH v2] cxl/hdm: allow zero sized committed decoders

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:
 <SN7PR12MB81317D99DDAB9680F10B58D6BBE4A@SN7PR12MB8131.namprd12.prod.outlook.com>
Date: Sat, 4 Oct 2025 13:30:29 +0000
From: Vishal Aslot <vaslot@...dia.com>
To: Gregory Price <gourry@...rry.net>
CC: Dave Jiang <dave.jiang@...el.com>, Davidlohr Bueso <dave@...olabs.net>,
	Jonathan Cameron <jonathan.cameron@...wei.com>, Alison Schofield
	<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, Ira
 Weiny <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>, Li Ming
	<ming.li@...omail.com>, Peter Zijlstra <peterz@...radead.org>, Dan Carpenter
	<dan.carpenter@...aro.org>, Zijun Hu <zijun.hu@....qualcomm.com>,
	"linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] cxl/hdm: allow zero sized committed decoders

> ________________________________________
> From: Gregory Price <gourry@...rry.net>
> Sent: Thursday, October 2, 2025 11:28 PM
> To: Vishal Aslot
> Cc: Dave Jiang; Davidlohr Bueso; Jonathan Cameron; Alison Schofield; Vishal Verma; Ira Weiny; > Dan Williams; Li Ming; Peter Zijlstra; Dan Carpenter; Zijun Hu; linux-cxl@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v2] cxl/hdm: allow zero sized committed decoders
>
> External email: Use caution opening links or attachments
>
>
> On Fri, Oct 03, 2025 at 12:59:07AM +0000, Vishal Aslot wrote:
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index e9e1d555cec6..50164fd1b434 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -1047,10 +1047,10 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>>                 }
>>
>> +               port->commit_end = cxld->id;
>> +
>
> Went looking to understand what commit_end actually does here, can you
> help explain?

My understanding is that "commit_end" is the high watermark inside a port with decoders. cxl_num_decoders_committed() returns this high watermark. In init_hdm_decoder(), there is a check to make sure that the next decoder is higher id by 1 (and must also be higher address). If not, it would error out saying "Committed out of order". 

Consider a case of 4 decoders (decoders 0 to 3 in xa_array/cxld->id). Decoder 0 is non-zero sized. The other 3 are zero-sized. All are committed.

In the last revision of the patch, I was doing "put_device()" on the zero-sized decoder, which shifted decoder ids to the left (ids 0, 1, 2, 3 became 0 -> 0, 1 -> deleted, 2 -> 1, 3 -> 2). This is fine and no "Committed out of order" error would be seen.

Now, I'm keeping the zero-sized decoders around, so I must update "commit_end". So I moved it up before the "if (size == 0)" check. This ensures no "Committed out of order" error would be seen because each decoder is one higher than the next.

>
>>                 if (size == 0) {
>> -                       dev_warn(&port->dev,
>> +                       dev_dbg(&port->dev,
>>                                  "decoder%d.%d: Committed with zero size\n",
>>                                  port->id, cxld->id);
>> -                       return -ENXIO;
>> +                       return -ENOSPC;
>>                 }
>> -               port->commit_end = cxld->id;
>>         } else {
>> @@ -1210,6 +1210,9 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>>                 rc = init_hdm_decoder(port, cxld, target_map, hdm, i,
>>                                       &dpa_base, info);
>>                 if (rc) {
>> +                       if (rc == -ENOSPC) {
>> +                               continue;
>> +                       }
>
> Don't need brackets here

Okie. Will remove it in v3.

>
>>                         dev_warn(&port->dev,
>>                                  "Failed to initialize decoder%d.%d\n",
>>                                  port->id, i);
>> --
>> 2.34.1